-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[CodeGen][ARM64EC] Don't treat guest exit thunks as indirect calls #165885
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@llvm/pr-subscribers-backend-aarch64 Author: Jacek Caban (cjacek) ChangesGuest exit thunks serve as glue for performing direct calls, so they shouldn’t treat the target as an indirect one. Spotted by @coneco-cy in #165504. Full diff: https://github.com/llvm/llvm-project/pull/165885.diff 2 Files Affected:
diff --git a/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp b/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp
index 1169f26a2ae37..97298f9d74171 100644
--- a/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp
@@ -655,16 +655,10 @@ Function *AArch64Arm64ECCallLowering::buildGuestExitThunk(Function *F) {
BasicBlock *BB = BasicBlock::Create(M->getContext(), "", GuestExit);
IRBuilder<> B(BB);
- // Load the global symbol as a pointer to the check function.
- Value *GuardFn;
- if (cfguard_module_flag == 2 && !F->hasFnAttribute("guard_nocf"))
- GuardFn = GuardFnCFGlobal;
- else
- GuardFn = GuardFnGlobal;
- LoadInst *GuardCheckLoad = B.CreateLoad(PtrTy, GuardFn);
-
- // Create new call instruction. The CFGuard check should always be a call,
- // even if the original CallBase is an Invoke or CallBr instruction.
+ // Create new call instruction. The call check should always be a call,
+ // even if the original CallBase is an Invoke or CallBr instructio.
+ // This is treated as a direct call, so do not use GuardFnCFGlobal.
+ LoadInst *GuardCheckLoad = B.CreateLoad(PtrTy, GuardFnGlobal);
Function *Thunk = buildExitThunk(F->getFunctionType(), F->getAttributes());
CallInst *GuardCheck = B.CreateCall(
GuardFnType, GuardCheckLoad, {F, Thunk});
diff --git a/llvm/test/CodeGen/WinCFGuard/cfguard-arm64ec.ll b/llvm/test/CodeGen/WinCFGuard/cfguard-arm64ec.ll
new file mode 100644
index 0000000000000..61c0431cd99a8
--- /dev/null
+++ b/llvm/test/CodeGen/WinCFGuard/cfguard-arm64ec.ll
@@ -0,0 +1,89 @@
+; REQUIRES: aarch64-registered-target
+; RUN: llc < %s -mtriple=arm64ec-windows | FileCheck %s
+
+; This file was generated from the following source, using this command line:
+; clang -target arm64ec-windows cfguard-arm64ec.c -S -emit-llvm -o cfguard-arm64ec.ll -O -Xclang -cfguard
+;
+;-------------------------------------------------------------------------------
+; extern void ext(void);
+;
+; void func(void (*f)())
+; {
+; ext();
+; f();
+; }
+;-------------------------------------------------------------------------------
+
+; ModuleID = 'cfguard-arm64ec.c'
+source_filename = "cfguard-arm64ec.c"
+target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-p:64:64-i32:32-i64:64-i128:128-n32:64-S128-Fn32"
+target triple = "arm64ec-unknown-windows-msvc19.33.0"
+
+; CHECK: "@feat.00" = 2048
+
+; Function Attrs: nounwind uwtable
+define dso_local void @func(ptr noundef readonly captures(none) %f) local_unnamed_addr #0 {
+entry:
+ tail call void @ext() #2
+; CHECK: bl "#ext"
+ tail call void %f() #2
+; CHECK-NEXT: adrp x8, __os_arm64x_check_icall_cfg
+; CHECK-NEXT: adrp x10, $iexit_thunk$cdecl$v$v
+; CHECK-NEXT: add x10, x10, :lo12:$iexit_thunk$cdecl$v$v
+; CHECK-NEXT: ldr x8, [x8, :lo12:__os_arm64x_check_icall_cfg]
+; CHECK-NEXT: mov x11, x19
+; CHECK-NEXT: blr x8
+ ret void
+}
+
+declare dso_local void @ext() local_unnamed_addr #1
+
+; CHECK: .def "#ext$exit_thunk";
+; CHECK-NEXT: .scl 2;
+; CHECK-NEXT: .type 32;
+; CHECK-NEXT: .endef
+; CHECK-NEXT: .section .wowthk$aa,"xr",discard,"#ext$exit_thunk"
+; CHECK-NEXT: .globl "#ext$exit_thunk" // -- Begin function #ext$exit_thunk
+; CHECK-NEXT: .p2align 2
+; CHECK-NEXT: "#ext$exit_thunk": // @"#ext$exit_thunk"
+; CHECK-NEXT: .weak_anti_dep ext
+; CHECK-NEXT: ext = "#ext"
+; CHECK-NEXT: .weak_anti_dep "#ext"
+; CHECK-NEXT: "#ext" = "#ext$exit_thunk"
+; CHECK-NEXT: .seh_proc "#ext$exit_thunk"
+; CHECK-NEXT: // %bb.0:
+; CHECK-NEXT: str x30, [sp, #-16]! // 8-byte Folded Spill
+; CHECK-NEXT: .seh_save_reg_x x30, 16
+; CHECK-NEXT: .seh_endprologue
+; CHECK-NEXT: adrp x8, __os_arm64x_check_icall
+; CHECK-NEXT: adrp x11, ext
+; CHECK-NEXT: add x11, x11, :lo12:ext
+; CHECK-NEXT: ldr x8, [x8, :lo12:__os_arm64x_check_icall]
+; CHECK-NEXT: adrp x10, $iexit_thunk$cdecl$v$v
+; CHECK-NEXT: add x10, x10, :lo12:$iexit_thunk$cdecl$v$v
+; CHECK-NEXT: blr x8
+; CHECK-NEXT: .seh_startepilogue
+; CHECK-NEXT: ldr x30, [sp], #16 // 8-byte Folded Reload
+; CHECK-NEXT: .seh_save_reg_x x30, 16
+; CHECK-NEXT: .seh_endepilogue
+; CHECK-NEXT: br x11
+; CHECK-NEXT: .seh_endfunclet
+; CHECK-NEXT: .seh_endproc
+
+attributes #0 = { nounwind uwtable "frame-pointer"="reserved" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="generic" "target-features"="+fp-armv8,+neon,+v8a,-fmv" }
+attributes #1 = { "frame-pointer"="reserved" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="generic" "target-features"="+fp-armv8,+neon,+v8a,-fmv" }
+attributes #2 = { nounwind }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5, !6, !7}
+!llvm.ident = !{!8}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, producer: "clang version 22.0.0git", isOptimized: true, runtimeVersion: 0, emissionKind: NoDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "cfguard-arm64ec.c", directory: "/home/jacek/vbox-shared")
+!2 = !{i32 2, !"cfguard", i32 2}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"wchar_size", i32 2}
+!5 = !{i32 8, !"PIC Level", i32 2}
+!6 = !{i32 7, !"uwtable", i32 2}
+!7 = !{i32 7, !"frame-pointer", i32 3}
+!8 = !{!"clang version 22.0.0git"}
|
| ; CHECK-NEXT: adrp x8, __os_arm64x_check_icall_cfg | ||
| ; CHECK-NEXT: adrp x10, $iexit_thunk$cdecl$v$v | ||
| ; CHECK-NEXT: add x10, x10, :lo12:$iexit_thunk$cdecl$v$v | ||
| ; CHECK-NEXT: ldr x8, [x8, :lo12:__os_arm64x_check_icall_cfg] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I somehow missed adding a test for this when I originally implemented it? In any case, thanks for adding it.
Do we want to combine this test file with llvm/test/CodeGen/AArch64/cfguard-arm64ec.ll ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed that, I combined it in the new version.
efriedma-quic
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Guest exit thunks serve as glue for performing direct calls, so they shouldn’t treat the target as an indirect one. Spotted by @coneco-cy in llvm#165504.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/23/builds/15239 Here is the relevant piece of the build log for the reference |
Guest exit thunks serve as glue for performing direct calls, so they shouldn’t treat the target as an indirect one.
Spotted by @coneco-cy in #165504.